Conversation
github-actions
bot
added
new-cli
9999years
force-pushed
the
structured-type-error
branch
3 times, most recently
from
2a62e05 to
776e652
Compare
9999years
force-pushed
the
structured-type-error
branch
from
776e652 to
1de04fd
Compare
| state.autoCallFunction(autoArgs, *v, *vNew); | ||
| v = vNew; | ||
| state.forceValue(*v, noPos); | ||
| state.forceValue(*v, pos); |
| } else | ||
| return nullptr; | ||
| //error<TypeError>("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); | ||
| //error<EvalError>("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); |
There was a problem hiding this comment.
| //error<EvalError>("'%s' is not an attribute set", getAttrPathStr()).debugThrow(); | |
| //error<TypeError>("nAttrs, getAttrPathStr()).debugThrow(); |
Is this better?
There was a problem hiding this comment.
Oh I guess getAttrPathStr is not the value, however
There was a problem hiding this comment.
Still, unless we are sure we never catch a TypeError, I am a bit wary of changing this
| return s->first; | ||
| } else | ||
| root->state.error<TypeError>("'%s' is not a string", getAttrPathStr()).debugThrow(); | ||
| // TODO: Is this an internal error? Where is literally any documentation for the eval cache? |
There was a problem hiding this comment.
There probably is no documentation :(
From seeing how this is used, I think this cursor thing can either just traverse a regular "uncached" value, or traverse a cache entry's serialized value. The getString part is just the caller asking for a specific type; it's not an internal error.
| [[nodiscard, gnu::noinline]] EvalErrorBuilder<T> & withFrameTrace(PosIdx pos, const std::string_view text); | ||
|
|
| std::string_view showType(ValueType type, bool article); | ||
|
|
There was a problem hiding this comment.
| std::string_view showType(ValueType type, bool article); |
It's in value.hh right?
| case nExternal: return WA("an", "external value"); | ||
| case nFloat: return WA("a", "float"); | ||
| case nThunk: return WA("a", "thunk"); | ||
| } |
There was a problem hiding this comment.
If we stick with macro
| } | |
| } | |
| #undef WA |
|
|
||
| std::string_view showType(ValueType type, bool withArticle) | ||
| { | ||
| #define WA(a, w) withArticle ? a " " w : w |
There was a problem hiding this comment.
I wonder if constexpr stuff would work for this or not
| // Allow selecting a subset of enum values | ||
| #pragma GCC diagnostic push | ||
| #pragma GCC diagnostic ignored "-Wswitch-enum" |
There was a problem hiding this comment.
Can we just list the ones we want to combine below with follow-through? There is a attribute thing to say "this follow-through is intentional".
Ericson2314
left a comment
Ericson2314
left a comment
There was a problem hiding this comment.
Sorry with my traveling I did not see this sooner. Just a few small things / questions. Excited to see this work continue!
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-03-11-nix-team-meeting-132/42960/1 |
3 participants
Motivation
With #9834 merged, we can now start converting stringly-typed errors (which just contain a string message) to structured errors (which contain the data to format the string message).
This is good because it means we can just change the formatting code in one place. Structured error classes increase the consistency of error messages and makes refactors like #9754 much easier. Structured error classes also helps us clarify our exception hierarchy. (I've found several separate classes of error while preparing this PR — I've converted these to
EvalErrorfor now and will leave them for future PRs.) Finally, structured error classes lay the groundwork for unique error codes (like many other programming languages support).Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.